Added allowance for argument zero_terms_query to be any case. Allowed… by MitchellGale · Pull Request #95 · Bit-Quill/opensearch-project-sql

MitchellGale · 2022-07-26T16:08:28Z

Signed-off-by: Mitchell Gale mitchellg@bitquilltech.com

Description

Allows for any case on argument name for relevancy based function (Match Phrase Prefix, Simple Query String, Match Bool Prefix, Match, Multi-Match, Query String, Relevance, Simple Query String).
e.g.
AnalyZer can work instead of just analyzer
Zero_Terms_Query can work instead of just zero_terms_query
BOOst can work instead of just boost

Allows for zero_term_query arguments to be of any case.
e.g.
All can work instead of all

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

acarbonetto · 2022-07-26T16:41:44Z

...ava/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java

    while (iterator.hasNext()) {
      NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next();
-      if (!queryBuildActions.containsKey(arg.getArgName())) {
+      String argNormalized = arg.getArgName().toLowerCase();


The isLowerCase may fail if arg.getArgName() doesn't exist.

...st/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java

Yury-Fridlyand

SQL/PPL parser is already case-insensitive. The goal is to change case for some parameter values.
See example from main:

opensearchsql> select * from logs where simple_query_string(['*'], 'CLR', AnAlYZer = 'standard');
Output longer than terminal width
Do you want to display data vertically for better visual effect? [y/N]: y
opensearchsql> select * from logs where simple_query_string(['*'], 'CLR', analyzer = 'stAndaRd');
TransportError(500, 'SearchPhaseExecutionException', {'error': {'type': 'SearchPhaseExecutionException', 'reason': 'Error occurred in OpenSearch engine: all shards failed', 'details': 'Shard[0]: [logs/uHvAufvyREut9LnosEX2yg] QueryShardException[[simple_query_string] analyzer [stAndaRd] not found]\n\nFor more details, please send request for Json format to see the raw response from OpenSearch engine.'}, 'status': 503})

So all calls for analyzer's value should be wrapped by .toLower():

.put("analyzer", (b, v) -> b.analyzer(v.stringValue()))

like you did for ZeroTermsQuery and Flags. Boost has double type, nothing to convert there, but what about Operator, DefaultOperator and so on?

...a/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java

MaxKsyunz · 2022-07-26T20:59:18Z

@MitchellGale-BitQuill the conflicts need to be resolved and SQL Java CI should pass as well.

… is surrounded by same type of quote (opensearch-project#696) Signed-off-by: mitchellg <mitchellg@bitquilltech.com> Co-authored-by: Andrew Carbonetto <andrewc@AndrewsMBP2022.local>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

… outside of main OS sql project (opensearch-project#704) Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

* Add support for highlight to parser and AstExpressionBuilder Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add unit test for highlight in AstExpressionBuilder Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add unit test for highlight in AstBuilderTest Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Support highlight as an Unresolved expression. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Represent highlight as UnresolvedExpression. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Support highlight in Analyzer. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Treat highlight as a proper function in AST In particular, highlightField is an expression now. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add support for highlight in Analyzer HighlightFunction is converted to LogicalHighlight logical plan. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add a simple IT test for highlight. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Register highlight function in the BuiltInFunctionRepository Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Partial support for highlight in physical plan. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add HighlightOperator. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Highlight alpha complete. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Initial implementation to upporting highlight('*') Signed-off-by: forestmvey <forestv@bitquilltech.com> * Add support for multiple highlight calls in select statement. Signed-off-by: forestmvey <forestv@bitquilltech.com> * Removed OpenSearchLogicalIndexScan highlightFields and dependencies. Improved test coverage and fixing checkstyle errors. Signed-off-by: forestmvey <forestv@bitquilltech.com> * Added HighlightExpressionTest Signed-off-by: forestmvey <forestv@bitquilltech.com> * Added javadocs, minor PR revisions, and fixed jacoco errors by improving coverage for OpenSearchIndexScan Signed-off-by: forestmvey <forestv@bitquilltech.com> * Code cleanup, adding parsing failure tests, and adding tests for highlight acceptance as a string literal as well as qualified name. Signed-off-by: forestmvey <forestv@bitquilltech.com> * Removing HighlightOperator functionality and unnecessary visitHighlight call in PhysicalPlanNodeVisitor.. Signed-off-by: forestmvey <forestv@bitquilltech.com> * Adding highlight function to functions.rst and removing unecessary function call in OpenSearchIndexScan. Signed-off-by: forestmvey <forestv@bitquilltech.com> * Change highlight fields returned format to array list. Changed highlight all and wildcard to unsupported to open up output formatting changes for multiple returned highlight fields. Change tests for updated coverage and disable highlight all and wildcard tests. Signed-off-by: forestmvey <forestv@bitquilltech.com> * Fix bug where invalid schema name was being used for returned highlight fields Signed-off-by: forestmvey <forestv@bitquilltech.com> * Fix failing integration tests due to schema changes for highlight expression type. Signed-off-by: forestmvey <forestv@bitquilltech.com> Co-authored-by: MaxKsyunz <maxk@bitquilltech.com>

codecov · 2022-08-05T18:11:14Z

Codecov Report

Merging #95 (8c49d05) into Integ-RelFunc_changeCaseWhereAppropriate (5c6fd72) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@                              Coverage Diff                               @@
##             Integ-RelFunc_changeCaseWhereAppropriate      #95      +/-   ##
==============================================================================
+ Coverage                                       97.74%   97.77%   +0.02%     
- Complexity                                       2858     2882      +24     
==============================================================================
  Files                                             273      276       +3     
  Lines                                            7020     7086      +66     
  Branches                                          442      447       +5     
==============================================================================
+ Hits                                             6862     6928      +66     
  Misses                                            157      157              
  Partials                                            1        1

Flag	Coverage Δ
sql-engine	`97.77% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ensearch/sql/planner/physical/PhysicalPlanDSL.java	`100.00% <ø> (ø)`
.../sql/planner/physical/PhysicalPlanNodeVisitor.java	`100.00% <ø> (ø)`
...ain/java/org/opensearch/sql/analysis/Analyzer.java	`100.00% <100.00%> (ø)`
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java	`100.00% <100.00%> (ø)`
...org/opensearch/sql/analysis/HighlightAnalyzer.java	`100.00% <100.00%> (ø)`
...ensearch/sql/expression/ExpressionNodeVisitor.java	`100.00% <100.00%> (ø)`
...opensearch/sql/expression/HighlightExpression.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/BuiltinFunctionName.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/OpenSearchFunctions.java	`100.00% <100.00%> (ø)`
...ensearch/sql/planner/logical/LogicalHighlight.java	`100.00% <100.00%> (ø)`
... and 14 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

… is surrounded by same type of quote (opensearch-project#696) Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

…ed functions Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

…ed functions Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

…ub.com/Bit-Quill/opensearch-project-sql into dev-RelFunc_changeCaseWhereAppropriate

MitchellGale · 2022-08-09T16:04:10Z

#103 PR remade in 103

MitchellGale requested review from MaxKsyunz, Yury-Fridlyand, acarbonetto and forestmvey July 26, 2022 16:08

acarbonetto reviewed Jul 26, 2022

View reviewed changes

...st/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java Show resolved Hide resolved

acarbonetto reviewed Jul 26, 2022

View reviewed changes

...st/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java Show resolved Hide resolved

acarbonetto approved these changes Jul 26, 2022

View reviewed changes

Yury-Fridlyand suggested changes Jul 26, 2022

View reviewed changes

MaxKsyunz requested changes Jul 26, 2022

View reviewed changes

...a/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java Outdated Show resolved Hide resolved

MitchellGale and others added 4 commits August 2, 2022 14:23

Two single or double quote escapes single or double quote when string…

68663d1

… is surrounded by same type of quote (opensearch-project#696) Signed-off-by: mitchellg <mitchellg@bitquilltech.com> Co-authored-by: Andrew Carbonetto <andrewc@AndrewsMBP2022.local>

Remove BWC tests (opensearch-project#721)

d26cde3

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Github Actions fix for reference to OpenSearch-Dashboard not existing…

6a99b56

… outside of main OS sql project (opensearch-project#704) Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

MitchellGale force-pushed the dev-RelFunc_changeCaseWhereAppropriate branch from 534aa05 to 145b0c5 Compare August 5, 2022 17:57

MitchellGale and others added 13 commits August 5, 2022 11:41

Two single or double quote escapes single or double quote when string…

ed3ed80

… is surrounded by same type of quote (opensearch-project#696) Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>

Remove BWC tests (opensearch-project#721)

5a6e779

Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>

Merged from upstream

9532614

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merged from upstream

15e318e

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added additional lower case force to simple query string query.

7922f2d

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added additional method to handle stringValue and toUpper

64b0114

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Used method for valueOfToUpper.

1d0b38f

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Used method for valueOfToUpper.

0046f05

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added lower case in MultiMatchQuery and QueryStringQuery.

d5728bb

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Removed extra spaces.

de1ffec

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merged from upstream

8024e90

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merged from upstream

0511f91

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Removed upper case force on analyzer.

73dfdf1

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

MitchellGale added 6 commits August 5, 2022 11:42

Merged from upstream

1e92dbe

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added toLower case in analyzer for relevancy based functions.

2ffb4c7

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

removed and added some tolower case and toupper case in relevancy bas…

fdb0d6c

…ed functions Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added space between valueOfUpper and valuetoLower

dd24131

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Fixed broken test after lower case is given to paramters.

2569be5

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Fixed invalid query for ztp.

6b2802a

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

MitchellGale force-pushed the dev-RelFunc_changeCaseWhereAppropriate branch from 145b0c5 to 6b2802a Compare August 5, 2022 18:43

MitchellGale added 18 commits August 5, 2022 12:09

Merged from upstream

e37dcd3

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merged from upstream

087fd5a

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added additional lower case force to simple query string query.

a8bf1fc

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added additional method to handle stringValue and toUpper

ec67718

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Used method for valueOfToUpper.

86e6fa1

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Used method for valueOfToUpper.

c240e9d

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added lower case in MultiMatchQuery and QueryStringQuery.

30eb62b

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Removed extra spaces.

f5125fc

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merged from upstream

fae3b07

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merged from upstream

f54fad4

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Removed upper case force on analyzer.

9b027e8

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merged from upstream

f4fe0dc

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added toLower case in analyzer for relevancy based functions.

3f86f2f

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

removed and added some tolower case and toupper case in relevancy bas…

8fcb35e

…ed functions Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Added space between valueOfUpper and valuetoLower

43c20a6

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Fixed broken test after lower case is given to paramters.

4062e34

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Fixed invalid query for ztp.

5104aee

Signed-off-by: mitchellg <mitchellg@bitquilltech.com>

Merge branch 'dev-RelFunc_changeCaseWhereAppropriate' of https://gith…

8c49d05

…ub.com/Bit-Quill/opensearch-project-sql into dev-RelFunc_changeCaseWhereAppropriate

MitchellGale closed this Aug 9, 2022

Yury-Fridlyand deleted the dev-RelFunc_changeCaseWhereAppropriate branch January 12, 2023 19:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added allowance for argument zero_terms_query to be any case. Allowed…#95

Added allowance for argument zero_terms_query to be any case. Allowed…#95
MitchellGale wants to merge 41 commits intoInteg-RelFunc_changeCaseWhereAppropriatefrom
dev-RelFunc_changeCaseWhereAppropriate

MitchellGale commented Jul 26, 2022

Uh oh!

acarbonetto Jul 26, 2022

Uh oh!

Uh oh!

Uh oh!

Yury-Fridlyand left a comment

Uh oh!

Uh oh!

MaxKsyunz commented Jul 26, 2022

Uh oh!

codecov bot commented Aug 5, 2022 •

edited

Loading

Uh oh!

MitchellGale commented Aug 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

MitchellGale commented Jul 26, 2022

Description

Check List

Uh oh!

acarbonetto Jul 26, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MaxKsyunz commented Jul 26, 2022

Uh oh!

codecov bot commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MitchellGale commented Aug 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Aug 5, 2022 •

edited

Loading